-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add circuits for KYC enforcement with anonymity #25
Conversation
…hdraw Signed-off-by: Jim Zhang <[email protected]>
Signed-off-by: Jim Zhang <[email protected]>
Signed-off-by: Jim Zhang <[email protected]>
Signed-off-by: Jim Zhang <[email protected]>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. Thanks for integrating Codecov - We've got you covered ☂️ |
…ble tokens with KYC Signed-off-by: Jim Zhang <[email protected]>
Signed-off-by: Jim Zhang <[email protected]>
/// @dev returns whether the given public key is registered | ||
/// @param publicKey The Babyjubjub public key to check | ||
/// @return bool whether the given public key is included in the registry | ||
function isRegistered( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure where this gets used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I answered my own question: it doesn't need to be used directly. as the circuit does the check with the identitiesRoot https://github.com/hyperledger-labs/zeto/pull/25/files#diff-9383275728425055b13bcbc860ce1ca0d4885b23049fa3ecf52fa71f1f144e65R105
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it's a convenient method for client apps. not essential for the circuit verifier to work.
import { UTXO, User, newUser, newUTXO, newNullifier, doMint, ZERO_UTXO, parseUTXOEvents } from './lib/utils'; | ||
import { loadProvingKeys, prepareDepositProof, prepareNullifierWithdrawProof } from './utils'; | ||
|
||
describe("Zeto based fungible token with anonymity, KYC, using nullifiers without encryption", function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should there be a test for unregistered user checks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ZKP part covered in https://github.com/hyperledger-labs/zeto/pull/25/files#diff-16c442cf7ed2d5adf832bff0e695df6a69fd1d71081cb25cad880fc93cd9ca79R146
I think it's worth having one at the contract level to capture regressions that are outside circuits.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added two new tests for the above suggested cases
zkp/js/test/anon_nullifier_kyc.js
Outdated
error = e; | ||
} | ||
expect(error).to.match(/Error in template Zeto_254 line: 126/); | ||
expect(error).to.match(/Error in template CheckSMTProof_253 line: 42/); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
expect(error).to.match(/Error in template CheckSMTProof_253 line: 42/); | |
expect(error).to.match(/Error in template CheckSMTProof_253 line: 46/); |
function deposit( | ||
uint256 amount, | ||
uint256 utxo, | ||
Commonlib.Proof calldata proof | ||
) public { | ||
_deposit(amount, utxo, proof); | ||
uint256[] memory utxos = new uint256[](1); | ||
utxos[0] = utxo; | ||
_mint(utxos); | ||
} | ||
|
||
function withdraw( | ||
uint256 amount, | ||
uint256[2] memory nullifiers, | ||
uint256 output, | ||
uint256 root, | ||
Commonlib.Proof calldata proof | ||
) public { | ||
_withdrawWithNullifiers(amount, nullifiers, output, root, proof); | ||
processInputsAndOutputs(nullifiers, [output, 0]); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these 2 functions be allowed if the sender is not in the registry?
-- my understanding is because of "nullifiers", UTXO created by un-registered users will not be able to circulate, so that might be the reason these two functions do not check against the registry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct, even if an unregistered user did a deposit, the only thing that user can do is return it back out via withdraw because the UTXOs are not allowed to be transferred.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @jimthematrix I left a few comments.
I love the refactor of the circuit code, they are awesome!!! 🚀 make the code so much easier to read. However, I didn't check the difference line by line as I'm assuming no modification was required while you broke them into small pieces for reusability.
…thdraw Signed-off-by: Jim Zhang <[email protected]>
@jimthematrix #25 (comment) needs addressing before merging the PR, otherwise, the circuit tests will fail. Maybe the circuit tests should be part of the pipeline now as we've got automation in place? |
Signed-off-by: Jim Zhang <[email protected]>
thanks @Chengxuan. sorry about missing that. just pushed another commit to update the error checking. and yes #30 adds github actions that runs all the tests |
Signed-off-by: Jim Zhang <[email protected]>
A sample solution that enforces KYC on the sender and receiver Babyjubjub keys, by using a merkle tree in the smart contract to capture the registered identities, so that the transactions can prove against the current merkle tree root.